refactor(router-core): Process routeTree into segment tree instead of flatRoutes #5722
Conversation
WalkthroughReplaces the legacy pathname parser and route-tree processor with a new segment-trie processor, moves parsing/matching primitives into a new module, updates path utilities to use the segment model, replaces RouterCore's flatRoutes with processedTree and simplifies getMatchedRoutes, updates HMR and tests accordingly, and marks MatchRouteOptions.caseSensitive as deprecated. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant RouterCore
participant NewProc as new-process-route-tree
participant PathUtil as path.ts
rect rgb(230,245,234)
Note over RouterCore,NewProc: Build processed route tree
Dev->>RouterCore: buildRouteTree(routeTree)
RouterCore->>NewProc: processRouteTree(routeTree, caseSensitive)
NewProc->>NewProc: parseSegment / build trie
NewProc-->>RouterCore: processedTree
end
rect rgb(245,245,230)
Note over PathUtil,NewProc: Interpolation uses segment model
Dev->>PathUtil: interpolatePath / resolvePath(...)
PathUtil->>NewProc: parseSegment (traverse pattern)
NewProc-->>PathUtil: segment node data
PathUtil-->>Dev: final path
end
rect rgb(230,245,234)
Note over RouterCore,NewProc: Runtime matching
Dev->>RouterCore: getMatchedRoutes(pathname)
RouterCore->>NewProc: findRouteMatch(pathname, processedTree)
NewProc->>NewProc: trie traversal -> extractParams
NewProc-->>RouterCore: matchedRoutes + foundRoute + params
RouterCore-->>Dev: match result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit b6570e3
☁️ Nx Cloud last updated this comment at |
…not have said param in returned params object
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/router-core/src/path.ts (1)
129-129: Unusual cache key separator—confirm this is intentional.The cache key uses
\\\\as a separator between base and to paths. While this avoids conflicts with forward slashes, it's unconventional. If this choice was deliberate to prevent edge-case collisions, consider adding a brief inline comment explaining the rationale.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-core/src/path.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/router-core/src/path.ts
📚 Learning: 2025-09-22T00:56:49.237Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Applied to files:
packages/router-core/src/path.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Applied to files:
packages/router-core/src/path.ts
📚 Learning: 2025-09-22T00:56:53.426Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Applied to files:
packages/router-core/src/path.ts
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
packages/router-core/src/path.ts
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Applied to files:
packages/router-core/src/path.ts
🧬 Code graph analysis (1)
packages/router-core/src/path.ts (4)
packages/router-core/src/lru-cache.ts (1)
LRUCache(1-5)packages/router-core/src/utils.ts (1)
last(191-193)packages/router-core/src/new-process-route-tree.ts (6)
parseSegment(32-128)SegmentKind(11-15)SEGMENT_TYPE_PATHNAME(6-6)SEGMENT_TYPE_PARAM(7-7)SEGMENT_TYPE_WILDCARD(8-8)SEGMENT_TYPE_OPTIONAL_PARAM(9-9)packages/router-generator/src/utils.ts (1)
cleanPath(38-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (3)
packages/router-core/src/path.ts (3)
316-316: The 'undefined' string fallback is intentional and tested behavior.Tests in
packages/router-core/tests/path.test.tsexplicitly verify that missing required parameters produce the literal string'undefined'(lines 279–289). The interpolation also correctly setsisMissingParams = trueto signal incomplete paths.For comparison, optional parameters fall back to empty string, and splat parameters omit the segment entirely when missing. This design allows graceful degradation while flagging via
isMissingParamsthat the path is incomplete.The current behavior appears intentional and working as designed. However, verify whether this is the intended design for the PR or if a different approach (throwing errors or omitting segments) is preferred for required parameters.
117-207: Verify test status with actual test execution before merge.Tests for URL param encoding exist in
packages/router-core/tests/path.test.ts(including URI component encoding cases withencodeURIComponentand customdecodeCharMaphandling). However, I cannot execute the test suite to confirm whether tests are currently passing or if any are actually failing.Run the unit tests locally to verify the status of URL param encoding tests:
pnpm run test:unitEnsure all path-related tests pass before merging, particularly the URI encoding tests in the
interpolatePathdescribe block.
194-201: The reported serialization issue does not exist—code is functioning correctly.Analysis of the code at lines 194-201 confirms:
prefixandsuffixderive fromsubstring()calls (lines 192-193), which always return strings, neverundefined- The optional-param template literal
${prefix}{-$${value}}${suffix}produces exactly one$character before the interpolated value: the{-$contains a literal$(not interpolation), followed by${value}interpolation, creating{-$value}at runtime—not double-$or "undefined" artifacts- Test coverage confirms correct behavior across all scenarios (100+ tests in optional-path-params.test.ts, curly-params-smoke.test.ts, and others show expected output without corruption)
No fixes needed.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/router-core/src/path.ts (1)
105-205:resolvePathdrops trailing slash for wildcard + empty_splatwhentrailingSlash: 'always'For a template like
/splat/$andtrailingSlash: 'always',resolvePathcurrently:
- Splits to
['', 'splat', '$', ''],- Appends
'/'unconditionally wheni > 0, even for the final empty segment,- Produces
/splat//$, whichcleanPathcollapses to/splat/$.Later, when
_splatis missing/empty,interpolatePathremoves the wildcard segment and returns/splatwith no trailing/, contradicting the expectation that'always'yields/splat/(see the new test inpath.test.ts).You can fix this by making the join loop sensitive to leading/trailing empty segments instead of blindly inserting
'/'for every index:- let segment - let joined = '' - for (let i = 0; i < baseSegments.length; i++) { - if (i > 0) joined += '/' - const part = baseSegments[i]! - if (!part) continue - segment = parseSegment(part, 0, segment) - const kind = segment[0] - if (kind === SEGMENT_TYPE_PATHNAME) { - joined += part - continue - } - const end = segment[5] - const prefix = part.substring(0, segment[1]) - const suffix = part.substring(segment[4], end) - const value = part.substring(segment[2], segment[3]) - if (kind === SEGMENT_TYPE_PARAM) { - joined += prefix || suffix ? `${prefix}{$${value}}${suffix}` : `$${value}` - } else if (kind === SEGMENT_TYPE_WILDCARD) { - joined += prefix || suffix ? `${prefix}{$}${suffix}` : '$' - } else { - // SEGMENT_TYPE_OPTIONAL_PARAM - joined += `${prefix}{-$${value}}${suffix}` - } - } + let segment + let joined = '' + for (let i = 0; i < baseSegments.length; i++) { + const part = baseSegments[i]! + const isFirst = i === 0 + const isLast = i === baseSegments.length - 1 + + if (!part) { + // Leading empty segment -> ensure leading '/' + if (isFirst && !joined) { + joined = '/' + } else if ( + isLast && + trailingSlash === 'always' && + !joined.endsWith('/') + ) { + // Trailing empty segment encodes the requested trailing slash + joined += '/' + } + continue + } + + if (!isFirst && !joined.endsWith('/')) { + joined += '/' + } + + segment = parseSegment(part, 0, segment) + const kind = segment[0] + if (kind === SEGMENT_TYPE_PATHNAME) { + joined += part + continue + } + const end = segment[5] + const prefix = part.substring(0, segment[1]) + const suffix = part.substring(segment[4], end) + const value = part.substring(segment[2], segment[3]) + if (kind === SEGMENT_TYPE_PARAM) { + joined += prefix || suffix ? `${prefix}{$${value}}${suffix}` : `$${value}` + } else if (kind === SEGMENT_TYPE_WILDCARD) { + joined += prefix || suffix ? `${prefix}{$}${suffix}` : '$' + } else { + // SEGMENT_TYPE_OPTIONAL_PARAM + joined += `${prefix}{-$${value}}${suffix}` + } + }This preserves existing behavior for static/param segments, but for templates like
/splat/$it yields/splat/$/, and after interpolation with_splat === ''the final path becomes/splat/as intended.packages/router-core/src/router.ts (1)
2478-2533:matchRouteignores router-levelcaseSensitiveoptionIn the new implementation:
const match = findSingleMatch( next.pathname, opts?.caseSensitive ?? false, opts?.fuzzy ?? false, baseLocation.pathname, this.processedTree, )
caseSensitivedefaults tofalsewhenever the caller doesn’t pass it, even if the router was configured withcaseSensitive: true. Previously, route matching (and thereforematchRoute) respected the router-levelcaseSensitivedefault.This can silently change behavior for apps that rely on case-sensitive routing but call
matchRoute/useMatchRoutewithout explicitly specifyingcaseSensitive.A minimal fix is to fall back to the router option:
- const match = findSingleMatch( - next.pathname, - opts?.caseSensitive ?? false, - opts?.fuzzy ?? false, - baseLocation.pathname, - this.processedTree, - ) + const match = findSingleMatch( + next.pathname, + opts?.caseSensitive ?? this.options.caseSensitive ?? false, + opts?.fuzzy ?? false, + baseLocation.pathname, + this.processedTree, + )Everything else in
matchRoute(deep-equality check forlocation.params, and optional search comparison) looks consistent with prior behavior.
♻️ Duplicate comments (1)
packages/router-core/src/new-process-route-tree.ts (1)
353-372: Add stable tie-breaker for deterministic route ordering.The sortDynamic function returns 0 when two segments have identical prefix, suffix, and case-sensitivity properties. This creates non-deterministic ordering when multiple routes share these attributes, which can cause inconsistent matching behavior across runs or JavaScript engines.
Add a final alphabetical comparison by fullPath to ensure stable, deterministic ordering:
if (a.suffix && !b.suffix) return -1 if (!a.suffix && b.suffix) return 1 if (a.caseSensitive && !b.caseSensitive) return -1 if (!a.caseSensitive && b.caseSensitive) return 1 + // Stable tie-breaker for deterministic ordering + return a.fullPath.localeCompare(b.fullPath) - return 0 }
🧹 Nitpick comments (2)
packages/router-core/tests/optional-path-params-clean.test.ts (1)
15-51: SharedparsePathname/PathSegmenthelpers could be centralizedThis
PathSegment+parsePathnamecombo is effectively identical to the one inoptional-path-params.test.ts. Consider extracting a tiny test utility (e.g.test-utils/segment-helpers.ts) to keep these helpers in sync and make future parser changes easier to roll out across tests.packages/router-core/src/new-process-route-tree.ts (1)
510-522: Remove commented code before final merge.The commented-out
intoRouteLikefunction should be removed if it's no longer needed, or uncommented and documented if it will be used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/router-core/src/new-process-route-tree.ts(1 hunks)packages/router-core/src/path.ts(4 hunks)packages/router-core/src/router.ts(10 hunks)packages/router-core/tests/new-process-route-tree.test.ts(1 hunks)packages/router-core/tests/optional-path-params-clean.test.ts(2 hunks)packages/router-core/tests/optional-path-params.test.ts(5 hunks)packages/router-core/tests/path.test.ts(18 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/router-core/tests/new-process-route-tree.test.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/router-core/tests/path.test.tspackages/router-core/src/path.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/tests/optional-path-params-clean.test.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/src/router.ts
📚 Learning: 2025-09-22T00:56:49.237Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Applied to files:
packages/router-core/tests/path.test.tspackages/router-core/src/path.tspackages/router-core/src/new-process-route-tree.ts
📚 Learning: 2025-10-14T18:59:33.990Z
Learnt from: FatahChan
Repo: TanStack/router PR: 5475
File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0
Timestamp: 2025-10-14T18:59:33.990Z
Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.
Applied to files:
packages/router-core/tests/path.test.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/tests/optional-path-params-clean.test.ts
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Applied to files:
packages/router-core/tests/path.test.tspackages/router-core/src/path.tspackages/router-core/tests/optional-path-params.test.tspackages/router-core/tests/optional-path-params-clean.test.tspackages/router-core/src/new-process-route-tree.ts
📚 Learning: 2025-09-22T00:56:53.426Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Applied to files:
packages/router-core/tests/path.test.tspackages/router-core/src/path.tspackages/router-core/src/new-process-route-tree.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Applied to files:
packages/router-core/src/path.tspackages/router-core/tests/optional-path-params-clean.test.tspackages/router-core/src/new-process-route-tree.tspackages/router-core/src/router.ts
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
packages/router-core/src/new-process-route-tree.tspackages/router-core/src/router.ts
🧬 Code graph analysis (6)
packages/router-core/tests/path.test.ts (2)
packages/router-core/src/path.ts (2)
interpolatePath(247-349)resolvePath(116-206)packages/router-core/src/new-process-route-tree.ts (7)
processRouteTree(645-702)findSingleMatch(595-615)SegmentKind(11-15)parseSegment(54-151)SEGMENT_TYPE_PATHNAME(6-6)SEGMENT_TYPE_PARAM(7-7)SEGMENT_TYPE_WILDCARD(8-8)
packages/router-core/src/path.ts (4)
packages/router-core/src/lru-cache.ts (1)
LRUCache(1-5)packages/router-core/src/utils.ts (1)
last(191-193)packages/router-core/src/new-process-route-tree.ts (5)
parseSegment(54-151)SEGMENT_TYPE_PATHNAME(6-6)SEGMENT_TYPE_PARAM(7-7)SEGMENT_TYPE_WILDCARD(8-8)SEGMENT_TYPE_OPTIONAL_PARAM(9-9)packages/router-generator/src/utils.ts (1)
cleanPath(38-41)
packages/router-core/tests/optional-path-params.test.ts (1)
packages/router-core/src/new-process-route-tree.ts (8)
SegmentKind(11-15)parseSegment(54-151)SEGMENT_TYPE_PATHNAME(6-6)SEGMENT_TYPE_OPTIONAL_PARAM(9-9)SEGMENT_TYPE_PARAM(7-7)SEGMENT_TYPE_WILDCARD(8-8)processRouteTree(645-702)findSingleMatch(595-615)
packages/router-core/tests/optional-path-params-clean.test.ts (1)
packages/router-core/src/new-process-route-tree.ts (6)
SegmentKind(11-15)parseSegment(54-151)SEGMENT_TYPE_PATHNAME(6-6)SEGMENT_TYPE_OPTIONAL_PARAM(9-9)processRouteTree(645-702)findSingleMatch(595-615)
packages/router-core/src/new-process-route-tree.ts (2)
packages/router-core/src/route.ts (3)
path(1551-1553)RouteLike(1968-1979)fullPath(1555-1557)packages/router-core/src/lru-cache.ts (2)
LRUCache(1-5)createLRUCache(7-74)
packages/router-core/src/router.ts (5)
packages/router-core/src/new-process-route-tree.ts (7)
ProcessedTree(539-554)processRouteTree(645-702)processRouteMasks(556-573)findFlatMatch(578-590)findSingleMatch(595-615)trimPathRight(637-639)findRouteMatch(617-634)packages/router-core/src/lru-cache.ts (1)
createLRUCache(7-74)packages/router-core/src/route.ts (2)
path(1551-1553)RouteMask(1466-1475)packages/router-core/src/path.ts (3)
resolvePath(116-206)cleanPath(25-28)trimPathRight(38-40)packages/router-core/src/utils.ts (1)
deepEqual(333-387)
🔇 Additional comments (15)
packages/router-core/tests/optional-path-params.test.ts (2)
15-55: LocalparsePathnamecorrectly tracksparseSegmentoutputThe helper cleanly mirrors
parseSegment’s index contract (type/value/prefix/suffix) and reuses the buffer, so these tests are exercising the new segment parser accurately without extra allocations. Looks good.
334-354:matchPathnamewrapper preserves legacymatchPathnamebehaviorWiring
findSingleMatch(options.to, …, from, processedTree)is consistent with the new function’s(from, …, path)signature and the legacy{ to }-based API. Returningundefinedonly whentois provided and no match is found also matches previous semantics.packages/router-core/tests/optional-path-params-clean.test.ts (1)
141-161:matchPathnametest harness is consistent with the new single-route matcherThe helper correctly seeds a minimal
processedTreeand delegates matching tofindSingleMatch, so these tests validate the new matcher rather than the old path helpers. No issues spotted.packages/router-core/tests/path.test.ts (1)
362-395: Trailing-slash interpolation tests align withinterpolatePathbehaviorThese new cases around required vs optional params and trailing
/give good coverage of the updated interpolation logic, including optional segments with and without values. They match the current implementation’s intent.packages/router-core/src/path.ts (1)
221-347:interpolatePath+encodeParamlogic looks soundThe new cursor-based interpolation using
parseSegmentcorrectly distinguishes pathname, param, optional param, and wildcard segments, and:
- Tracks
usedParamsandisMissingParamsconsistently (including the_splatmissing cases the tests assert on),- Leaves
/unencoded for_splatviaencodeURI, while routing all other params throughencodePathParam+decodeCharMap,- Handles optional params with prefix/suffix by omitting just the param when the value is
null/undefined.Given the existing tests around numeric params, URI encoding, missing params, and optional segments, the behavior here matches expectations.
packages/router-core/src/router.ts (4)
1212-1221:resolvePathWithBasecache wiring is straightforwardUsing a dedicated
resolvePathCacheLRU and passing it intoresolvePathis a nice way to avoid recomputing path resolutions. The cache key intentionally omitstrailingSlashbased on the assumption that a router instance uses a fixedtrailingSlashoption, which matches how this cache is used here.
1453-1551:getMatchedRoutesintegration withProcessedTreelooks correct
RouterCore.getMatchedRoutesnow delegates to the exportedgetMatchedRouteshelper with{ pathname, routesById, processedTree }. The helper usesfindRouteMatch(trimPathRight(pathname), processedTree, true)and falls back to the root route when no match is found, preserving the previous fuzzy-root semantics. Param propagation intorouteParamsviaObject.assignalso guards against mutating cached match objects.
1782-1797: Route mask matching viafindFlatMatchis well-structuredUsing
findFlatMatch<RouteMask<TRouteTree>>(next.pathname, this.processedTree)and copyingmatch.paramsinto a freshparamsobject avoids mutating cached matches, and destructuring{ from: _from, ...maskProps }to buildmaskedDestkeeps the publicfromfield out of the masked navigation options. Behavior-wise this aligns with the existing route-masking model.
2626-2655:getMatchedRouteshelper correctly switches tofindRouteMatchThe exported
getMatchedRoutesnow usesfindRouteMatch(trimPathRight(pathname), processedTree, true)instead of flat-route parsing, then walksparentRoutelinks to buildmatchedRoutes, defaulting to the root route when no match is found. This is a clean adaptation to the newProcessedTreerepresentation and matches the router’s existing fuzzy-not-found behavior.packages/router-core/src/new-process-route-tree.ts (6)
1-151: Well-structured parsing foundation.The segment parsing logic is clean and efficient. The use of Uint16Array for parsed segment representation is a smart performance optimization, and the regex patterns correctly handle the braced parameter syntax. The parseSegment function properly handles all segment types with appropriate index boundary tracking.
162-351: Solid recursive tree-building logic.The parseSegments function correctly handles tree construction with proper depth tracking, node reuse, and case sensitivity. The logic to distinguish between layout routes and index routes (lines 329-337) is well thought out, and the recursive child processing maintains correct parent-child relationships.
405-455: Well-structured node creation with performance considerations.The node creation functions properly initialize all fields, and the comment about field ordering for engine optimization shows attention to performance details. The type structure using discriminated unions is clean and type-safe.
556-702: Comprehensive tree processing with proper caching.The tree processing functions correctly handle both tree-based and flat route structures. The LRU caching strategy is well-applied, and the depth initialization is consistent throughout. The routesByPath logic that prioritizes index routes (line 679) is correct for handling ambiguous cases.
724-801: Parameter extraction logic is well-structured.The extractParams and buildBranch functions correctly handle all segment types with proper decoding. The lazy initialization of nodeParts and the bitmask optimization for optional params show good attention to performance.
The URL encoding complexities (matching encoded path parts against unencoded route prefixes/suffixes) are inherently challenging and likely the source of the failing tests mentioned in the PR objectives.
803-1055: Sophisticated matching algorithm with correct priority handling.The stack-based matching approach efficiently explores all possible routes with proper priority ordering. The use of statics/dynamics/optionals counters to track match quality is elegant, and the bitmask optimization for skipped optional params is clever. The wildcard suffix check (line 919) correctly handles multi-segment remaining paths by joining and checking the trailing characters.
This is a complex but well-designed algorithm that correctly implements the intended matching semantics.
Sheraff
deleted the
refactor-router-core-process-route-tree-into-segment-tree
branch
Sheraff
changed the title
This PR rewrites
processRouteTreeto yield a trie-like tree of segments. This replacespackages/router-core/src/process-route-tree.tspackages/router-core/src/path.tsMatching complexity now grows linearly with the number of segments (i.e.
/a/b/c=> 3 segments), as opposed to the previous algorithm that was correlated with the number of routes.Goals:
Non-goals:
Fixes #5635
Fixes #5674
Fixes #5799
to-do:
/)Bench:
useMatchRoute)Notes:
useMatchRoute) does not care about therouteTree. It will match 2 arbitrary strings. This can lead to unexpected results, and is probably not the behavior we want in the long term.For now, this PR implements
findSingleMatchto preserve that behavior, and this method is slower than the current implementation. However, when we switch to matching against the routeTree (usingfindRouteMatchinstead), it will be faster.Future improvements:
find-my-way(router forfastify) and use nodes that are smaller than a full pathname segment. This yields a structure close to a radix tree, which could perform better.Summary by CodeRabbit
Deprecations
Breaking Changes
New Features
Tests